Support vanilla CI - #1
Merged
Merged
Conversation
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op18-49 #1. The served worker always bound the IPv4 wildcard, while the cell view publishes the pod IP and wraps it in brackets when that IP is IPv6. On an IPv6-only cluster clients therefore dialled an address the worker had no listening socket for. Bind the dual-stack wildcard wherever the platform offers one, and keep the IPv4 wildcard where IPv6 is unavailable.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op28-3 #2, op28-14 #3, op28-6 #1 A trainer controller's pool is named trainer-controller-<trainer id>, and a pool name may hold 40 characters, so a legal 16 character model id already overflows through the -critic suffix the run generates itself. Validation only checked uniqueness and DNS syntax, so the run failed while building Helm values rather than while reading its config. Derive the trainer id budget from the prefix and the pool name limit, and check every resolved trainer id against it.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op24-10 #1 A deployment that runs no object store master only had to name an address containing a colon, so `master:not-a-port` and ports outside 1-65535 passed. That path never reaches the launcher code that parses the port, so the helm install succeeded and every trainer worker of the deployment then failed to initialize mooncake.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op32-2 #1. Both scripts import their shared address book as `examples.…`, and running a file by path puts its own directory on `sys.path` instead of the repository root, so the documented command died on `ModuleNotFoundError: No module named 'examples'` before installing anything.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op28-3 #2, op28-14 #3, op28-6 #1 A trainer controller's pool is named trainer-controller-<trainer id>, and a pool name may hold 40 characters, so a legal 16 character model id already overflows through the -critic suffix the run generates itself. Validation only checked uniqueness and DNS syntax, so the run failed while building Helm values rather than while reading its config. Derive the trainer id budget from the prefix and the pool name limit, and check every resolved trainer id against it.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op18-49 #1. The served worker always bound the IPv4 wildcard, while the cell view publishes the pod IP and wraps it in brackets when that IP is IPv6. On an IPv6-only cluster clients therefore dialled an address the worker had no listening socket for. Bind the dual-stack wildcard wherever the platform offers one, and keep the IPv4 wildcard where IPv6 is unavailable.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-148 #1. The diagnosis listed every release of the namespace and only then kept the ones of this run. helm list truncates to its 256-release maximum before returning, so a busy namespace can drop the sibling trainer or inference release of the very run that failed. Listing now passes a name filter, which helm applies before that maximum.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-150 #1. Every driver released the worker manager on its last line, so a raise from training, evaluation, or any of the dispose calls left the Ray CommandActor without a shutdown request and the run without its process trees reaped. Each driver now owns the manager in a thin wrapper and runs the rest of the script under a finally.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-156 #1. add_note landed in Python 3.11, and setup.py still claims 3.10 support, so on 3.10 the attempt to annotate the primary failure raised AttributeError from the error path itself and replaced the very failure it was describing. The annotation now follows the capability check the rest of the repo uses, and falls back to logging the secondary traceback.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-25 #1. The job ignores every DisruptionTarget failure so an eviction that displaced a pod before it ran does not spend the single attempt, but the same rule covers a pod evicted after its container started: Kubernetes then creates a replacement and runs the command a second time, doubling whatever it wrote. Fail the job first on a container that terminated non-zero, which a disrupted pod's killed container did and a pod displaced while pending never has, so the ignore rule keeps meaning what it was written for.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op24-10 #1 A deployment that runs no object store master only had to name an address containing a colon, so `master:not-a-port` and ports outside 1-65535 passed. That path never reaches the launcher code that parses the port, so the helm install succeeded and every trainer worker of the deployment then failed to initialize mooncake.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-57 #1. The rpc variant of the short gsm8k test ran the shared body, which always passed the mooncake object store arguments. RayObjectStore is built with frees_objects=True only when the run pairs the ray object store with the rpc worker comm backend, so no e2e test ever put an object through it. The shared body now picks the object store by comm backend: the rpc variant asks for the ray object store, every other variant keeps mooncake. The rpc entry drops its mooncake label, which no longer describes what it runs.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-81 #1. The rollout recovery witness accepted any reading that reported the cell Serving long enough after its last injection. A cell carries its state and its health as separate fields, so a cell whose Healthy condition is False or Unknown can still report Serving for a while, and one such reading was enough to call a dead-but-unregistered replica recovered. A reading now counts only when the cell is alive and Serving.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-141 #1. The stall dumper registered the SIGTERM faulthandler on the default file descriptor 2, which pytest's fd capture points at a per-test temp file whose contents are only reported once the test finishes. A SIGTERM ends pytest before that, so the traceback that says where a killed run was stuck never reached CI. The dumper now takes a duplicate of the real stderr, made while global capture is suspended, and registers the handler on that descriptor.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-165 #1. The split deployment scenario ran its target side as one release per deployment, but the pipeline released every side through the default handoff, which rebuilds a single release name from the side's original ALL config. That name belongs to no release the split side installed, so nothing waited for the TRAINER, INFERENCE and PRIMARY releases, and the PRIMARY one, which run_split_training leaves installed, could still hold gpus after the comparison had been read. The pipeline now takes a release_side of its own, and the split scenario passes one that removes every release its deployment list names, in reverse install order, waiting for each as the default handoff does.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-141 #1. The stall dumper registered the SIGTERM faulthandler on the default file descriptor 2, which pytest's fd capture points at a per-test temp file whose contents are only reported once the test finishes. A SIGTERM ends pytest before that, so the traceback that says where a killed run was stuck never reached CI. The dumper now takes a duplicate of the real stderr, made while global capture is suspended, and registers the handler on that descriptor.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-165 #1. The split deployment scenario ran its target side as one release per deployment, but the pipeline released every side through the default handoff, which rebuilds a single release name from the side's original ALL config. That name belongs to no release the split side installed, so nothing waited for the TRAINER, INFERENCE and PRIMARY releases, and the PRIMARY one, which run_split_training leaves installed, could still hold gpus after the comparison had been read. The pipeline now takes a release_side of its own, and the split scenario passes one that removes every release its deployment list names, in reverse install order, waiting for each as the default handoff does.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-57 #1. The rpc variant of the short gsm8k test ran the shared body, which always passed the mooncake object store arguments. RayObjectStore is built with frees_objects=True only when the run pairs the ray object store with the rpc worker comm backend, so no e2e test ever put an object through it. The shared body now picks the object store by comm backend: the rpc variant asks for the ray object store, every other variant keeps mooncake. The rpc entry drops its mooncake label, which no longer describes what it runs.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-81 #1. The rollout recovery witness accepted any reading that reported the cell Serving long enough after its last injection. A cell carries its state and its health as separate fields, so a cell whose Healthy condition is False or Unknown can still report Serving for a while, and one such reading was enough to call a dead-but-unregistered replica recovered. A reading now counts only when the cell is alive and Serving.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-141 #1. The stall dumper registered the SIGTERM faulthandler on the default file descriptor 2, which pytest's fd capture points at a per-test temp file whose contents are only reported once the test finishes. A SIGTERM ends pytest before that, so the traceback that says where a killed run was stuck never reached CI. The dumper now takes a duplicate of the real stderr, made while global capture is suspended, and registers the handler on that descriptor.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-165 #1. The split deployment scenario ran its target side as one release per deployment, but the pipeline released every side through the default handoff, which rebuilds a single release name from the side's original ALL config. That name belongs to no release the split side installed, so nothing waited for the TRAINER, INFERENCE and PRIMARY releases, and the PRIMARY one, which run_split_training leaves installed, could still hold gpus after the comparison had been read. The pipeline now takes a release_side of its own, and the split scenario passes one that removes every release its deployment list names, in reverse install order, waiting for each as the default handoff does.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op27-30 #1 Resolves op27-17 #3 A take-over that finds no checkpoint zeroes the scheduler and resets the optimizer, and only then checks the state it rebuilt against the moments adam keeps. A Muon member rebuilds a `momentum_buffer`, so the check refused it after the trainer had already been mutated, and the surviving trainer was left half changed. Refuse the optimizer among the other preconditions, before anything is touched.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op18-49 #1. The served worker always bound the IPv4 wildcard, while the cell view publishes the pod IP and wraps it in brackets when that IP is IPv6. On an IPv6-only cluster clients therefore dialled an address the worker had no listening socket for. Bind the dual-stack wildcard wherever the platform offers one, and keep the IPv4 wildcard where IPv6 is unavailable.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-25 #1. The job ignores every DisruptionTarget failure so an eviction that displaced a pod before it ran does not spend the single attempt, but the same rule covers a pod evicted after its container started: Kubernetes then creates a replacement and runs the command a second time, doubling whatever it wrote. Fail the job first on a container that terminated non-zero, which a disrupted pod's killed container did and a pod displaced while pending never has, so the ignore rule keeps meaning what it was written for.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op32-5 #1. The hot restart process assertions only counted complete snapshots and demanded half the observation attempts answer, so a run whose first freeze was never observed could satisfy both from the t1 and t2 snapshots alone. The trainer identity those compare against would then be the one the first take-over installed, which is exactly what the assertion exists to rule out. Demand at least one whole-release snapshot that carries the trainer boot uuid and was taken before any workload was stamped.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op32-6 #1 and op35-135 #1. The reward gate this specification promises was deliberately replaced by calibrated eval gates, and those live in the long solver-verifier test, not in the three-rollout deployment test. TRAIN_REWARD_BOUNDS and its assertion no longer exist anywhere, so the specification claimed a guarantee that nothing could enforce. Say what the test actually asserts and where learning is gated instead.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op32-6 #2 and op35-155 #1. The specification claims the hot restart deterministic comparison runs with no exemption, while _compare passes the four rollout/weight_version statistics to exclude_keys. The exclusion is right - the trainer is the process a take-over keeps alive, so its update counter keeps counting through the steps the target redoes - but a specification that hides it overstates what the regression covers. Write the exemption and its reason down.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op32-21 #1. generate_data is the one generated subcommand still declaring mode as a required option, so typer rejects the command before resolve_mode_fn is reached and None never gets there. The two deterministic deployment scenarios pin their topology through a resolve_mode_fn that ignores its argument, and their users still have to spell a --mode that decides nothing. Declare it as the OptionalModeOption the other subcommands use, and phrase the real-rollout assertion off the resolved mode.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op32-26 #1 and op32-14 #1. The observer treats any listing holding at least one pod and one workload as a whole release, and the realistic soak starts it before the Helm install. A release still coming up is therefore recorded, its missing workloads leave empty pod-name sets, and the pods created next read as pods a take-over replaced - a healthy run failing on install timing. Record nothing until two consecutive reads agree on the release's workloads and pods, then drop any listing that lost a settled workload without the release being gone.
fzyzcjy
added a commit
that referenced
this pull request
Sep 2, 2026
Resolves op35-156 #1. add_note landed in Python 3.11, and setup.py still claims 3.10 support, so on 3.10 the attempt to annotate the primary failure raised AttributeError from the error path itself and replaced the very failure it was describing. The annotation now follows the capability check the rest of the repo uses, and falls back to logging the secondary traceback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
p.s. wandb can be seen here: https://wandb.ai/ch271828n-team/projects